Skip to content

ref: remove client side data truncation#1903

Merged
Litarnus merged 5 commits into
5.xfrom
martinl/sdk-truncation
Sep 18, 2025
Merged

ref: remove client side data truncation#1903
Litarnus merged 5 commits into
5.xfrom
martinl/sdk-truncation

Conversation

@Litarnus
Copy link
Copy Markdown
Contributor

@Litarnus Litarnus commented Sep 15, 2025

It looks like this is the only place we perform SDK-side data truncation that isn't related to recursion depth or other limits mentioned in #1814.

closes PHP-1

Comment on lines -399 to -414
public function testLongString(bool $serializeAllObjects): void
{
$serializer = $this->createSerializer();

if ($serializeAllObjects) {
$serializer->setSerializeAllObjects(true);
}

foreach ([100, 1000, 1010, 1024, 1050, 1100, 10000] as $length) {
$input = str_repeat('x', $length);
$result = $this->invokeSerialization($serializer, $input);

$this->assertIsString($result);
$this->assertLessThanOrEqual(1024, \strlen($result));
}
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also change this to $this->assertLessThanOrEqual($length, \strlen($result)); which is admittedly a bit goofy but it would test that truncation is not applied.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed this would be a bit goofy. Don't think we need to test this specifically.

@linear
Copy link
Copy Markdown

linear Bot commented Sep 15, 2025

@Litarnus Litarnus marked this pull request as ready for review September 15, 2025 07:00
@Litarnus Litarnus changed the base branch from master to 5.x September 15, 2025 09:18
@cleptric cleptric added the 5.0 label Sep 16, 2025
Comment on lines -399 to -414
public function testLongString(bool $serializeAllObjects): void
{
$serializer = $this->createSerializer();

if ($serializeAllObjects) {
$serializer->setSerializeAllObjects(true);
}

foreach ([100, 1000, 1010, 1024, 1050, 1100, 10000] as $length) {
$input = str_repeat('x', $length);
$result = $this->invokeSerialization($serializer, $input);

$this->assertIsString($result);
$this->assertLessThanOrEqual(1024, \strlen($result));
}
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed this would be a bit goofy. Don't think we need to test this specifically.

@Litarnus Litarnus merged commit 422a317 into 5.x Sep 18, 2025
38 checks passed
@Litarnus Litarnus deleted the martinl/sdk-truncation branch September 18, 2025 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove unnecessary SDK-side data truncation and rely on Relay for handling max length limits

3 participants